Skip to content

feat: add artist-release-editorial template files#125

Merged
sweetmantech merged 14 commits intomainfrom
feat/artist-release-editorial-template
Apr 8, 2026
Merged

feat: add artist-release-editorial template files#125
sweetmantech merged 14 commits intomainfrom
feat/artist-release-editorial-template

Conversation

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator

@recoup-coding-agent recoup-coding-agent commented Apr 7, 2026

Summary

  • Adds artist-release-editorial template directory with all required files
  • New test: loadArtistReleaseEditorial.test.ts

Test plan

  • Unit test: template loads with correct data
  • Template loads at runtime
  • Reference images to be added when available

🤖 Generated with Claude Code


Summary by cubic

Adds the artist-release-editorial template and improves image/overlay handling. Auto-selects a face guide, routes non-face images as attachments, overlays them top-left in the final video, and hardens ffmpeg rendering and shared utils for reliability.

  • New Features

    • New artist-release-editorial template with style/caption guides, video moods/movements, and caption examples; usesFaceGuide: true, usesImageOverlay: true, and a style guide that generates only an editorial artist portrait (no compositing).
    • Face-detected routing: resolveFaceGuide returns { faceGuideUrl, additionalImageUrls }; first face becomes faceGuideUrl, others become attachments; if none and usesFaceGuide is true, fetch face-guide.png from GitHub.
    • Image/video pipeline: generateContentImage appends additionalImageUrls after face guide and reference (deduped and self-deduped); renderFinalVideo downloads overlays, caps at 6, and stacks them from the top-left via buildFilterComplex and ffmpeg filter_complex.
  • Refactors & Fixes

    • SRP/DRY: extracted buildFfmpegArgs, calculateCaptionLayout, stripEmoji, wrapText; split classifyImages; added shared downloadImageBuffer; trimmed renderFinalVideo.
    • detectFace uses exact label matching to avoid false positives and logs failures.
    • buildFilterComplex emits [out] when no captions; downloadOverlayImages wraps per-image fetch in try/catch and skips failures.
    • renderFinalVideo fixes audio mapping in overlay path (-map 0:a in lipsync mode) and cleans up all temp files.
    • escapeDrawtext replaces quotes with U+2019 and fixes percent escaping to %% for reliable captions in filter_complex.
    • Validated usesImageOverlay with Zod in loadTemplate; unified logging with logStep.

Written for commit 928fbfc. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • New "artist-release-editorial" template: editorial image style, caption guidance/examples, and curated video moods/movements; supports image overlays.
    • Image generation and final video rendering now accept additional reference/overlay images and include them in composed outputs (overlay placement applied when template enables overlays).
  • Tests

    • Added suites covering template loading, face detection/selection, additional-image handling, overlay filter building, and image-generation flows.

Add template directory with style guide, caption guide, video moods,
video movements, and caption examples for editorial-style release promo
content featuring artist press photos, playlist covers, and DSP branding.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an "artist-release-editorial" template bundle, introduces face detection and a structured face-guide resolver that returns face and additional image URLs, extends image-generation and final rendering to accept/use extra overlay images (with ffmpeg overlay filter construction), and adds tests for these flows.

Changes

Cohort / File(s) Summary
Template bundle
src/content/templates/artist-release-editorial/style-guide.json, src/content/templates/artist-release-editorial/caption-guide.json, src/content/templates/artist-release-editorial/video-moods.json, src/content/templates/artist-release-editorial/video-movements.json, src/content/templates/artist-release-editorial/references/captions/examples.json
Adds the artist-release-editorial template: style guide (includes usesFaceGuide, usesImageOverlay, customInstruction, imagePrompt), caption guidance, examples, and arrays for video moods/movements.
Template loader
src/content/loadTemplate.ts, src/content/__tests__/loadArtistReleaseEditorial.test.ts
TemplateData now includes usesImageOverlay (parsed from style-guide); new test validates template fields and styleGuide.customInstruction.
Face detection & resolver
src/content/detectFace.ts, src/content/resolveFaceGuide.ts, src/content/__tests__/detectFace.test.ts, src/content/__tests__/resolveFaceGuide.test.ts
Adds detectFace(imageUrl): Promise<boolean> (calls falSubscribe). resolveFaceGuide now returns ResolveFaceGuideResult { faceGuideUrl, additionalImageUrls }, uploads each image, runs detectFace when appropriate, selects the first face image as faceGuideUrl, and collects others as additionalImageUrls. Tests updated for new behavior.
Image generation
src/content/generateContentImage.ts, src/content/__tests__/generateContentImage.test.ts
generateContentImage signature accepts additionalImageUrls?: string[]; appends these to image_urls passed to falSubscribe and logs hasAdditionalImages. Tests verify ordering and presence of appended URLs.
Task integration
src/tasks/createContentTask.ts
createContentTask destructures resolveFaceGuide result and passes additionalImageUrls into generateContentImage; passes overlayImageUrls into final render when template enables overlays.
Overlay processing & ffmpeg
src/content/buildOverlayFilters.ts, src/content/__tests__/buildOverlayFilters.test.ts, src/content/renderFinalVideo.ts
Adds buildOverlayFilters to generate ffmpeg -i inputs and a filterChain that scales and stacks overlays; renderFinalVideo downloads overlay images, skips failed downloads, and switches to -filter_complex with overlay chain + caption drawtext when overlays are present; otherwise keeps prior -vf flow. Tests validate filter construction.

Sequence Diagram(s)

sequenceDiagram
  participant Task as "createContentTask"
  participant Resolver as "resolveFaceGuide"
  participant Storage as "fal.storage.upload"
  participant Detect as "detectFace"
  participant ImgGen as "generateContentImage"
  participant Fal as "falSubscribe"
  participant Renderer as "renderFinalVideo"
  participant Fetch as "fetch"
  participant Builder as "buildOverlayFilters"

  rect rgba(200,230,255,0.5)
  Task->>Resolver: call resolveFaceGuide(images, usesFaceGuide, ...)
  end

  rect rgba(220,255,220,0.5)
  Resolver->>Storage: upload each image -> uploadedUrl
  Storage-->>Resolver: uploadedUrl
  Resolver->>Detect: detectFace(uploadedUrl) [if usesFaceGuide]
  Detect-->>Resolver: true/false
  Resolver-->>Task: { faceGuideUrl, additionalImageUrls }
  end

  rect rgba(255,240,200,0.5)
  Task->>ImgGen: call generateContentImage({ faceGuideUrl, referenceImagePath, prompt, additionalImageUrls })
  ImgGen->>Fal: falSubscribe({ image_urls: [faceGuide?, ref?, ...additionalImageUrls], prompt })
  Fal-->>ImgGen: job response (image URL)
  ImgGen-->>Task: return generated image URL
  end

  rect rgba(240,220,255,0.5)
  Task->>Renderer: call renderFinalVideo({ video, overlayImageUrls })
  Renderer->>Fetch: download each overlay URL -> temp file (skip failures)
  Fetch-->>Renderer: temp paths
  Renderer->>Builder: buildOverlayFilters(overlayPaths)
  Builder-->>Renderer: { inputs, filterChain }
  Renderer->>Fal: run ffmpeg with -filter_complex (overlay + drawtext)
  Fal-->>Renderer: final video
  Renderer-->>Task: final video path/URL
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I hop through JSON, find faces in a row,
I tuck extra pictures where soft overlays go,
Captions whisper low, and editors glow,
With tiny rabbit hops the pipeline starts to flow,
Hooray — the editorial carrots steal the show.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add artist-release-editorial template files' accurately and concisely describes the main change—adding template files for a new artist-release-editorial template.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/artist-release-editorial-template

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/content/__tests__/loadArtistReleaseEditorial.test.ts (2)

23-29: Consider consolidating into a single test or making assertions more specific.

The second test re-loads the template to check one additional field. This could be combined with the first test, or the assertion could be more specific (e.g., checking minimum length or specific content patterns).

♻️ Option 1: Combine into single test
   it("loads the artist-release-editorial template", async () => {
     const template = await loadTemplate("artist-release-editorial");

     expect(template.name).toBe("artist-release-editorial");
     expect(template.imagePrompt).toBeTruthy();
     expect(template.usesFaceGuide).toBe(true);
     expect(template.styleGuide).not.toBeNull();
     expect(template.captionGuide).not.toBeNull();
     expect(template.videoMoods.length).toBeGreaterThan(0);
     expect(template.videoMovements.length).toBeGreaterThan(0);
     expect(template.captionExamples.length).toBeGreaterThan(0);
+
+    const sg = template.styleGuide as Record<string, unknown>;
+    expect(sg.customInstruction).toBeTruthy();
+    expect(typeof sg.customInstruction).toBe("string");
   });
-
-  it("has a customInstruction in the style guide", async () => {
-    const template = await loadTemplate("artist-release-editorial");
-    const sg = template.styleGuide as Record<string, unknown>;
-
-    expect(sg.customInstruction).toBeTruthy();
-    expect(typeof sg.customInstruction).toBe("string");
-  });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/__tests__/loadArtistReleaseEditorial.test.ts` around lines 23 -
29, Combine the redundant tests in loadArtistReleaseEditorial.test.ts by loading
the template once via loadTemplate("artist-release-editorial") and asserting
both that template.styleGuide exists and that styleGuide.customInstruction is a
non-empty string (e.g., typeof === "string" and length > 0 or matches a specific
pattern); update the test named "has a customInstruction in the style guide" to
perform both checks against template.styleGuide.customInstruction instead of
re-loading the template in a separate test.

10-21: Consider using Zod schema validation for more robust type checking.

The test uses manual truthiness checks and length assertions. Per coding guidelines for src/**/*.{ts,tsx}, Zod should be used for schema validation. A Zod schema would provide stronger guarantees about the template structure and automatically validate all fields.

📋 Example using Zod validation
+import { z } from "zod";
+
+const ArtistReleaseEditorialSchema = z.object({
+  name: z.literal("artist-release-editorial"),
+  imagePrompt: z.string().min(1),
+  usesFaceGuide: z.literal(true),
+  styleGuide: z.record(z.unknown()).nullable().refine(val => val !== null),
+  captionGuide: z.record(z.unknown()).nullable().refine(val => val !== null),
+  videoMoods: z.array(z.string()).min(1),
+  videoMovements: z.array(z.string()).min(1),
+  captionExamples: z.array(z.string()).min(1),
+  referenceImagePaths: z.array(z.string())
+});
+
 describe("loadTemplate artist-release-editorial", () => {
   it("loads the artist-release-editorial template", async () => {
-    const template = await loadTemplate("artist-release-editorial");
-
-    expect(template.name).toBe("artist-release-editorial");
-    expect(template.imagePrompt).toBeTruthy();
-    expect(template.usesFaceGuide).toBe(true);
-    expect(template.styleGuide).not.toBeNull();
-    expect(template.captionGuide).not.toBeNull();
-    expect(template.videoMoods.length).toBeGreaterThan(0);
-    expect(template.videoMovements.length).toBeGreaterThan(0);
-    expect(template.captionExamples.length).toBeGreaterThan(0);
+    const template = await loadTemplate("artist-release-editorial");
+    const result = ArtistReleaseEditorialSchema.safeParse(template);
+    
+    expect(result.success).toBe(true);
   });

As per coding guidelines: "Use Zod for schema validation" for src/**/*.{ts,tsx}.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/__tests__/loadArtistReleaseEditorial.test.ts` around lines 10 -
21, Replace the ad-hoc assertions in the "loads the artist-release-editorial
template" test with a Zod validation: import or define the appropriate Zod
schema (e.g., TemplateSchema or artistReleaseTemplateSchema) and run
schema.parse or schema.safeParse on the object returned by
loadTemplate("artist-release-editorial"); assert that validation succeeds (or
that safeParse().success is true) and optionally assert specific fields from the
parsed/typed result (e.g., parsed.template.name, parsed.template.imagePrompt,
parsed.template.usesFaceGuide, parsed.template.styleGuide,
parsed.template.captionGuide, parsed.template.videoMoods,
parsed.template.videoMovements, parsed.template.captionExamples) instead of
manual truthiness/length checks so the test enforces the full shape via Zod.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/content/__tests__/loadArtistReleaseEditorial.test.ts`:
- Around line 23-29: Combine the redundant tests in
loadArtistReleaseEditorial.test.ts by loading the template once via
loadTemplate("artist-release-editorial") and asserting both that
template.styleGuide exists and that styleGuide.customInstruction is a non-empty
string (e.g., typeof === "string" and length > 0 or matches a specific pattern);
update the test named "has a customInstruction in the style guide" to perform
both checks against template.styleGuide.customInstruction instead of re-loading
the template in a separate test.
- Around line 10-21: Replace the ad-hoc assertions in the "loads the
artist-release-editorial template" test with a Zod validation: import or define
the appropriate Zod schema (e.g., TemplateSchema or artistReleaseTemplateSchema)
and run schema.parse or schema.safeParse on the object returned by
loadTemplate("artist-release-editorial"); assert that validation succeeds (or
that safeParse().success is true) and optionally assert specific fields from the
parsed/typed result (e.g., parsed.template.name, parsed.template.imagePrompt,
parsed.template.usesFaceGuide, parsed.template.styleGuide,
parsed.template.captionGuide, parsed.template.videoMoods,
parsed.template.videoMovements, parsed.template.captionExamples) instead of
manual truthiness/length checks so the test enforces the full shape via Zod.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5ff15e58-8833-457f-aee3-b9790e2213ba

📥 Commits

Reviewing files that changed from the base of the PR and between e55101a and c8a2513.

📒 Files selected for processing (6)
  • src/content/__tests__/loadArtistReleaseEditorial.test.ts
  • src/content/templates/artist-release-editorial/caption-guide.json
  • src/content/templates/artist-release-editorial/references/captions/examples.json
  • src/content/templates/artist-release-editorial/style-guide.json
  • src/content/templates/artist-release-editorial/video-moods.json
  • src/content/templates/artist-release-editorial/video-movements.json

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/content/templates/artist-release-editorial/video-moods.json">

<violation number="1" location="src/content/templates/artist-release-editorial/video-moods.json:8">
P3: Typo: `its` → `it's`. This is the contraction of "it is," not the possessive.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

"name": "artist-release-editorial",
"description": "Editorial promo featuring artist press photo with playlist covers and DSP branding — the kind of polished-but-organic visual an artist's team drops alongside a new release",
"usesFaceGuide": true,
"customInstruction": "Generate an editorial-style press photo of the artist. The image should look like a professional press photo taken for a magazine feature or playlist editorial — polished lighting but still feeling authentic and personal. Do NOT composite playlist covers or logos into the AI-generated image. Those elements will be overlaid separately in post-production.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do NOT composite playlist covers or logos into the AI-generated image. Those elements will be overlaid separately in post-production.

That's not true. We will provide the playlist covers as attachments in the prompt. The image generation should use those attachments to match the references.

"description": "Editorial promo featuring artist press photo with playlist covers and DSP branding — the kind of polished-but-organic visual an artist's team drops alongside a new release",
"usesFaceGuide": true,
"customInstruction": "Generate an editorial-style press photo of the artist. The image should look like a professional press photo taken for a magazine feature or playlist editorial — polished lighting but still feeling authentic and personal. Do NOT composite playlist covers or logos into the AI-generated image. Those elements will be overlaid separately in post-production.",
"imagePrompt": "A professional editorial press photo of an artist. Clean, intentional lighting — soft key light with subtle rim light separation. The artist is posed naturally, looking directly at camera or slightly off-axis. The background is a solid or subtly textured surface (concrete wall, draped fabric, muted gradient) that does not distract from the subject. The mood is confident and polished but not sterile. Shot on a DSLR or medium format camera, shallow depth of field, cinematic color grade leaning warm or desaturated depending on the artist's aesthetic.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current prompt seems to deviate from our current vision

  • actual: generating the editorial photo without the logos / covers.
  • required: template takes the editorial photo + playlist covers + streaming logos and combines them into a unified image matching the reference.

… logos

- Style guide now instructs image generation to combine the artist
  press photo with playlist covers and streaming logos into a unified
  composite image, matching reference style
- Remove incorrect instruction about not compositing covers/logos
- Fix typo: its → it's in video-moods.json

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/content/templates/artist-release-editorial/style-guide.json">

<violation number="1" location="src/content/templates/artist-release-editorial/style-guide.json:5">
P2: The `customInstruction` tells the model to "match the layout and composition style shown in the reference images", but no reference images exist in the template yet. This will produce a misleading/broken prompt at runtime. Consider removing or rewording the reference-image clause until the images are actually added, or gate usage of this template on their presence.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

sweetmantech and others added 2 commits April 7, 2026 14:37
ref-01.png through ref-04.png, matching the pattern used by
artist-caption-bedroom template.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
generateContentImage now accepts optional additionalImageUrls that are
appended to the image_urls array sent to fal.ai. createContentTask
passes payload.images through so templates like artist-release-editorial
can receive album covers and playlist covers as model inputs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/content/__tests__/generateContentImage.test.ts (1)

55-99: Add one regression case for overlapping URLs.

Please add a test where additionalImageUrls contains the same URL as faceGuideUrl (or uploaded ref URL) to lock expected duplicate-handling behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/__tests__/generateContentImage.test.ts` around lines 55 - 99, Add
a regression test to generateContentImage that covers overlapping URLs: mock
fs.readFile and fal.storage.upload as in other tests, call generateContentImage
with faceGuideUrl and referenceImagePath (or mocked uploaded ref URL) and
include those same URLs inside additionalImageUrls, then assert via
mockFalSubscribe (mockFalSubscribe.mock.calls[0][1]) that callArgs.image_urls
contains a deduplicated list (unique URLs only, preserving the expected order)
so duplicates from additionalImageUrls are not repeated; reference
generateContentImage, mockFalSubscribe, fal.storage.upload, and fs.readFile to
locate where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/content/generateContentImage.ts`:
- Around line 51-53: Deduplicate additionalImageUrls before appending to
imageUrls: filter additionalImageUrls (the variable used in
generateContentImage.ts) to remove duplicates and any URLs already present in
imageUrls (and any faceGuideUrl/reference entries that are pushed earlier), then
push the unique ones; use a Set or equivalent to ensure both intra-additional
duplicates and cross-list duplicates are removed before calling
imageUrls.push(...additionalImageUrls).

---

Nitpick comments:
In `@src/content/__tests__/generateContentImage.test.ts`:
- Around line 55-99: Add a regression test to generateContentImage that covers
overlapping URLs: mock fs.readFile and fal.storage.upload as in other tests,
call generateContentImage with faceGuideUrl and referenceImagePath (or mocked
uploaded ref URL) and include those same URLs inside additionalImageUrls, then
assert via mockFalSubscribe (mockFalSubscribe.mock.calls[0][1]) that
callArgs.image_urls contains a deduplicated list (unique URLs only, preserving
the expected order) so duplicates from additionalImageUrls are not repeated;
reference generateContentImage, mockFalSubscribe, fal.storage.upload, and
fs.readFile to locate where to add the test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 271f8749-fb4b-4ed7-984d-eb5986fb6eb9

📥 Commits

Reviewing files that changed from the base of the PR and between d044bae and 6eeb506.

⛔ Files ignored due to path filters (4)
  • src/content/templates/artist-release-editorial/references/images/ref-01.png is excluded by !**/*.png
  • src/content/templates/artist-release-editorial/references/images/ref-02.png is excluded by !**/*.png
  • src/content/templates/artist-release-editorial/references/images/ref-03.png is excluded by !**/*.png
  • src/content/templates/artist-release-editorial/references/images/ref-04.png is excluded by !**/*.png
📒 Files selected for processing (3)
  • src/content/__tests__/generateContentImage.test.ts
  • src/content/generateContentImage.ts
  • src/tasks/createContentTask.ts

…tachments

resolveFaceGuide now analyzes each image in the payload using fal.ai
face detection to determine if it's a headshot or not:
- First face image → face guide
- Non-face images (album covers, playlist covers) → additionalImageUrls
- If no face found and usesFaceGuide is true → falls back to GitHub

This allows templates like artist-release-editorial to receive both the
face guide (from GitHub or payload) and album covers (as additional
model inputs) without hardcoding images[0] as the face guide.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/tasks/createContentTask.ts">

<violation number="1" location="src/tasks/createContentTask.ts:103">
P2: The first image in `payload.images` is already used as the face guide by `resolveFaceGuide`. Passing the full array here duplicates it in the `image_urls` sent to the model. Slice off the first element to avoid the duplicate.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

faceGuideUrl: faceGuideUrl ?? undefined,
referenceImagePath,
prompt: fullPrompt,
additionalImageUrls: payload.images,
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The first image in payload.images is already used as the face guide by resolveFaceGuide. Passing the full array here duplicates it in the image_urls sent to the model. Slice off the first element to avoid the duplicate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tasks/createContentTask.ts, line 103:

<comment>The first image in `payload.images` is already used as the face guide by `resolveFaceGuide`. Passing the full array here duplicates it in the `image_urls` sent to the model. Slice off the first element to avoid the duplicate.</comment>

<file context>
@@ -100,6 +100,7 @@ export const createContentTask = schemaTask({
       faceGuideUrl: faceGuideUrl ?? undefined,
       referenceImagePath,
       prompt: fullPrompt,
+      additionalImageUrls: payload.images,
     });
 
</file context>
Suggested change
additionalImageUrls: payload.images,
additionalImageUrls: payload.images?.slice(1),
Fix with Cubic

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/content/__tests__/resolveFaceGuide.test.ts">

<violation number="1" location="src/content/__tests__/resolveFaceGuide.test.ts:143">
P2: The second `mockResolvedValueOnce(true)` for `detectFace` is never consumed — `detectFace` is only called once here because the implementation skips face detection once `faceGuideUrl` is already set. Consider removing the unused mock and adding an assertion like `expect(detectFace).toHaveBeenCalledTimes(1)` to explicitly verify this behavior.</violation>
</file>

<file name="src/content/detectFace.ts">

<violation number="1" location="src/content/detectFace.ts:24">
P2: The caught error is discarded — log it so failures are diagnosable. Without the error, a rate-limit, auth failure, or schema change all look identical in production logs.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

.mockResolvedValueOnce("https://fal.ai/face2.png");
vi.mocked(detectFace)
.mockResolvedValueOnce(true)
.mockResolvedValueOnce(true);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The second mockResolvedValueOnce(true) for detectFace is never consumed — detectFace is only called once here because the implementation skips face detection once faceGuideUrl is already set. Consider removing the unused mock and adding an assertion like expect(detectFace).toHaveBeenCalledTimes(1) to explicitly verify this behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/content/__tests__/resolveFaceGuide.test.ts, line 143:

<comment>The second `mockResolvedValueOnce(true)` for `detectFace` is never consumed — `detectFace` is only called once here because the implementation skips face detection once `faceGuideUrl` is already set. Consider removing the unused mock and adding an assertion like `expect(detectFace).toHaveBeenCalledTimes(1)` to explicitly verify this behavior.</comment>

<file context>
@@ -97,4 +133,28 @@ describe("resolveFaceGuide", () => {
+      .mockResolvedValueOnce("https://fal.ai/face2.png");
+    vi.mocked(detectFace)
+      .mockResolvedValueOnce(true)
+      .mockResolvedValueOnce(true);
+
+    const result = await resolveFaceGuide({
</file context>
Fix with Cubic

Log count and URL prefixes of additional images to verify they're
being passed to the model.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
src/content/detectFace.ts (2)

18-21: Fragile type assertion on untyped API response.

The cast data.faces as unknown[] assumes the FAL API will always return a faces property. If the API response shape changes (e.g., faces becomes undefined or is renamed), Array.isArray(faces) will return false silently, which may be acceptable fail-safe behavior but could mask API contract changes.

Per coding guidelines, consider using Zod to validate the response shape and surface unexpected API changes early:

♻️ Optional: Add Zod schema validation
 import { logger } from "@trigger.dev/sdk/v3";
+import { z } from "zod";
 import { falSubscribe } from "./falSubscribe";
 
 const FACE_DETECTION_MODEL = "fal-ai/face-detection";
+
+const FaceDetectionResultSchema = z.object({
+  faces: z.array(z.unknown()).default([]),
+});
 
 export async function detectFace(imageUrl: string): Promise<boolean> {
   try {
     const result = await falSubscribe(FACE_DETECTION_MODEL, {
       image_url: imageUrl,
     });
 
-    const data = result.data as Record<string, unknown>;
-    const faces = data.faces as unknown[];
-
-    const hasFace = Array.isArray(faces) && faces.length > 0;
+    const parsed = FaceDetectionResultSchema.safeParse(result.data);
+    const hasFace = parsed.success && parsed.data.faces.length > 0;
     logger.log("Face detection result", { imageUrl: imageUrl.slice(0, 80), hasFace });
     return hasFace;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/detectFace.ts` around lines 18 - 21, The code in detectFace.ts
currently does a fragile cast of result.data to Record<string, unknown> and then
to unknown[] for faces (variables: data, faces, hasFace), which can silently
hide API contract changes; replace this with explicit runtime validation using a
Zod schema (e.g., define a schema that expects an object with an optional faces
array of objects) and parse/validate result.data before using it, then set
hasFace based on the validated output (or throw/log a clear error when
validation fails) so unexpected shapes are surfaced early.

24-29: Consider logging the error details for observability.

The bare catch block discards error details, which could make debugging production issues harder. Logging the error message would aid troubleshooting without changing the fail-safe behavior.

🔍 Optional: Log error details
-  } catch {
+  } catch (error) {
     logger.log("Face detection failed, assuming no face", {
       imageUrl: imageUrl.slice(0, 80),
+      error: error instanceof Error ? error.message : String(error),
     });
     return false;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/detectFace.ts` around lines 24 - 29, The catch block in
detectFace (where logger.log is called with imageUrl.slice(0, 80)) swallows
errors; change the bare catch to capture the error (e.g., catch (err)) and
include the error or err.message in the logger.log payload so the log records
the exception details while preserving the existing fail-safe behavior and
returning false.
src/content/resolveFaceGuide.ts (1)

38-51: Sequential image processing may slow down the pipeline.

Each image is uploaded and face-detected sequentially. For payloads with multiple images, this adds latency. Consider parallelizing the uploads (while keeping face detection sequential to preserve "first face wins" semantics):

⚡ Optional: Parallelize image uploads
   // Upload and classify each provided image
   if (images?.length) {
+    // Upload all images in parallel first
+    const uploadedUrls = await Promise.all(
+      images.map(imageUrl => fetchImageFromUrl(imageUrl))
+    );
+
+    // Then classify sequentially to preserve "first face wins"
-    for (const imageUrl of images) {
-      const uploadedUrl = await fetchImageFromUrl(imageUrl);
+    for (const uploadedUrl of uploadedUrls) {
       if (usesFaceGuide && !faceGuideUrl) {
         const hasFace = await detectFace(uploadedUrl);
         if (hasFace) {
           faceGuideUrl = uploadedUrl;
           continue;
         }
       }

       additionalImageUrls.push(uploadedUrl);
     }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/resolveFaceGuide.ts` around lines 38 - 51, The loop currently
calls fetchImageFromUrl and detectFace sequentially which adds latency; change
it to first upload all images in parallel (e.g., map images -> Promise of
fetchImageFromUrl and await Promise.all to get uploadedUrls in original order),
then iterate uploadedUrls sequentially and call detectFace(uploadedUrl) to
preserve the "first face wins" behavior, setting faceGuideUrl on the first
detected face and pushing others into additionalImageUrls; also handle
per-upload failures (skip or throw) so detectFace only runs on successful
uploads. Reference: images array, fetchImageFromUrl, detectFace, faceGuideUrl,
and additionalImageUrls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/content/detectFace.ts`:
- Around line 18-21: The code in detectFace.ts currently does a fragile cast of
result.data to Record<string, unknown> and then to unknown[] for faces
(variables: data, faces, hasFace), which can silently hide API contract changes;
replace this with explicit runtime validation using a Zod schema (e.g., define a
schema that expects an object with an optional faces array of objects) and
parse/validate result.data before using it, then set hasFace based on the
validated output (or throw/log a clear error when validation fails) so
unexpected shapes are surfaced early.
- Around line 24-29: The catch block in detectFace (where logger.log is called
with imageUrl.slice(0, 80)) swallows errors; change the bare catch to capture
the error (e.g., catch (err)) and include the error or err.message in the
logger.log payload so the log records the exception details while preserving the
existing fail-safe behavior and returning false.

In `@src/content/resolveFaceGuide.ts`:
- Around line 38-51: The loop currently calls fetchImageFromUrl and detectFace
sequentially which adds latency; change it to first upload all images in
parallel (e.g., map images -> Promise of fetchImageFromUrl and await Promise.all
to get uploadedUrls in original order), then iterate uploadedUrls sequentially
and call detectFace(uploadedUrl) to preserve the "first face wins" behavior,
setting faceGuideUrl on the first detected face and pushing others into
additionalImageUrls; also handle per-upload failures (skip or throw) so
detectFace only runs on successful uploads. Reference: images array,
fetchImageFromUrl, detectFace, faceGuideUrl, and additionalImageUrls.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6857b06-e61a-4d02-936d-8439c5502b6d

📥 Commits

Reviewing files that changed from the base of the PR and between 6eeb506 and 6ce2bd3.

📒 Files selected for processing (6)
  • src/content/__tests__/detectFace.test.ts
  • src/content/__tests__/resolveFaceGuide.test.ts
  • src/content/detectFace.ts
  • src/content/generateContentImage.ts
  • src/content/resolveFaceGuide.ts
  • src/tasks/createContentTask.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/content/generateContentImage.ts

Adds post-processing overlay of attached images (playlist covers,
streaming logos) onto the final video via ffmpeg filter_complex.

- Add usesImageOverlay flag to TemplateData (opt-in per template)
- artist-release-editorial enables usesImageOverlay
- buildOverlayFilters generates ffmpeg overlay filter chain
- renderFinalVideo downloads overlay images and composites them
  in the bottom-right corner, stacked vertically above captions
- createContentTask passes additionalImageUrls to renderFinalVideo
  only when the template opts in
- Fix detectFace to use Florence-2 object detection (fal-ai/face-detection
  model does not exist)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 9 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/content/buildOverlayFilters.ts">

<violation number="1" location="src/content/buildOverlayFilters.ts:40">
P2: The comment says placeholders are named `[OVR0], [OVR1], …` but the code actually emits `[ovr_in_0], [ovr_in_1], …` (plus `[video_base]` as the chain entry point). Update the comment to list the real label names so callers know which strings to map to input-stream indices.</violation>
</file>

<file name="src/content/renderFinalVideo.ts">

<violation number="1" location="src/content/renderFinalVideo.ts:304">
P2: `overlay.filterChain` is computed by `buildOverlayFilters` but never used — the overlay filter logic is manually duplicated inline. This means `buildOverlayFilters` can diverge from the actual rendering without any visible breakage. Consider either using the helper's `filterChain` (fixing its placeholder labels) or removing the helper's filter logic and keeping only `inputs`.</violation>

<violation number="2" location="src/content/renderFinalVideo.ts:357">
P0: Missing `-map 0:a` for the audio stream in the overlay + lipsync path. Once `-map [out]` is used for explicit stream selection, ffmpeg drops all unmapped streams. The `!hasAudio` branch correctly maps audio, but this branch only sets `-c:a aac` without mapping — producing a silent video.</violation>
</file>

<file name="src/content/detectFace.ts">

<violation number="1" location="src/content/detectFace.ts:7">
P2: Substring matching on `"man"` causes false positives for non-person labels like `"ottoman"` or `"mannequin"`. Use word-boundary matching or normalize to exact/whole-word checks to avoid spurious face detections.</violation>
</file>

<file name="src/content/__tests__/buildOverlayFilters.test.ts">

<violation number="1" location="src/content/__tests__/buildOverlayFilters.test.ts:37">
P2: This test claims to verify non-overlapping positions but only asserts `toBeTruthy()`, which is already guaranteed by any non-empty input. Consider asserting that the two `overlay=` expressions contain distinct Y coordinates (e.g. by extracting positions from the filter chain), or remove the test to avoid giving false confidence about positioning logic.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/content/__tests__/buildOverlayFilters.test.ts`:
- Around line 13-19: The tests for buildOverlayFilters currently only check for
the presence of "overlay=" and "scale=" which can pass despite incorrect
placement; update the assertions in the test cases (the one referencing
buildOverlayFilters at the top and the other around lines 33-37) to assert the
exact overlay coordinate clauses produced by buildOverlayFilters (e.g., assert
filterChain contains "overlay=<expectedX>:<expectedY>" or use a regex that
matches the precise x:y values) so the layout math is validated; keep using the
same function name buildOverlayFilters and assert the expected coordinates for
each scenario instead of the loose contains checks.

In `@src/content/loadTemplate.ts`:
- Around line 85-88: The current runtime type assertion for usesImageOverlay is
unsafe; replace it with Zod validation by defining a Zod schema (e.g.,
StyleGuideSchema) that specifies imagePrompt as optional string, usesFaceGuide
as boolean with default true, and usesImageOverlay as boolean with default
false, then safeParse/parse the styleGuide (the sg variable) inside the same
scope and extract imagePrompt, usesFaceGuide, and usesImageOverlay from the
validated result; update the code that currently sets
imagePrompt/usesFaceGuide/usesImageOverlay to use the parsed schema values so
string-"false" or other invalid values are rejected or coerced as intended.

In `@src/content/renderFinalVideo.ts`:
- Around line 340-345: The overlay stack overflows the frame when
overlayImagePaths is long; update the overlay loop in renderFinalVideo.ts to
compute a maximum number that fits the vertical space using FRAME_HEIGHT,
EDGE_PADDING, OVERLAY_SIZE and GAP (or alternatively compute scaled
OVERLAY_SIZE/GAP when count exceeds available space) and then iterate only over
Math.min(overlayImagePaths.length, maxOverlays) (or use the scaled sizes) so
computed y never becomes negative; adjust how outLabel/prevLabel/filterParts are
generated to reflect the reduced or scaled set so overlays render entirely
inside the frame.
- Around line 171-183: The downloaded overlay temp files in renderFinalVideo.ts
(overlayPaths array populated where overlay images are fetched) are never
deleted; update the cleanup in the function's finally block (the same place that
unlinks videoPath, audioPath, captionsPath, outputPath) to iterate overlayPaths
and unlink each file (safely ignoring missing files/errors) so per-run temp PNGs
are removed; reference the overlayPaths variable and the finally cleanup section
in renderFinalVideo to implement this.
- Around line 353-360: The video filter mapping "-map [out]" prevents ffmpeg
from auto-selecting audio, so in the renderFinalVideo logic when hasAudio is
true you must explicitly map the embedded audio (use the existing
audioInputIndex) before setting audio codec; update the branch that currently
only does args.push("-c:a", "aac") to also push args.push("-map",
`${audioInputIndex}:a:0`) (same mapping used in the else branch) so the audio
stream from input audioInputIndex is included when overlays/filtered video
output are used.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 601ec5b7-4ef9-46bd-8812-e076b4be52a9

📥 Commits

Reviewing files that changed from the base of the PR and between 6ce2bd3 and 414d946.

📒 Files selected for processing (9)
  • src/content/__tests__/buildOverlayFilters.test.ts
  • src/content/__tests__/detectFace.test.ts
  • src/content/__tests__/loadArtistReleaseEditorial.test.ts
  • src/content/buildOverlayFilters.ts
  • src/content/detectFace.ts
  • src/content/loadTemplate.ts
  • src/content/renderFinalVideo.ts
  • src/content/templates/artist-release-editorial/style-guide.json
  • src/tasks/createContentTask.ts
✅ Files skipped from review due to trivial changes (1)
  • src/content/tests/loadArtistReleaseEditorial.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/tasks/createContentTask.ts
  • src/content/tests/detectFace.test.ts
  • src/content/detectFace.ts
  • src/content/templates/artist-release-editorial/style-guide.json

Comment on lines +340 to +345
for (let i = 0; i < overlayImagePaths.length; i++) {
const x = FRAME_WIDTH - 150 - 30; // OVERLAY_SIZE - EDGE_PADDING
const yFromBottom = 30 + i * (150 + 20); // EDGE_PADDING + i * (OVERLAY_SIZE + GAP)
const y = FRAME_HEIGHT - 150 - 160 - yFromBottom;
const outLabel = i < overlayImagePaths.length - 1 ? `ovr_out_${i}` : "ovr_final";
filterParts.push(`[${prevLabel}][ovr_${i}]overlay=${x}:${y}[${outLabel}]`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The overlay stack runs off-frame after six images.

With the current constants, the seventh overlay computes y = -80, so extra playlist covers/logos render partially or fully outside the frame. additionalImageUrls can exceed that for editorial templates, so this needs a cap or a layout that scales to the available height.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/renderFinalVideo.ts` around lines 340 - 345, The overlay stack
overflows the frame when overlayImagePaths is long; update the overlay loop in
renderFinalVideo.ts to compute a maximum number that fits the vertical space
using FRAME_HEIGHT, EDGE_PADDING, OVERLAY_SIZE and GAP (or alternatively compute
scaled OVERLAY_SIZE/GAP when count exceeds available space) and then iterate
only over Math.min(overlayImagePaths.length, maxOverlays) (or use the scaled
sizes) so computed y never becomes negative; adjust how
outLabel/prevLabel/filterParts are generated to reflect the reduced or scaled
set so overlays render entirely inside the frame.

ffmpeg's filter_complex parser treats curly quotes (U+2019) as
delimiters, causing drawtext to fail when captions contain
apostrophes like "didn't". The new escapeDrawtext function replaces
all quote-like characters with modifier letter apostrophe (U+02BC)
which renders identically but is safe for ffmpeg.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/content/escapeDrawtext.ts">

<violation number="1" location="src/content/escapeDrawtext.ts:18">
P2: Percent escaping `%%%%` produces two `%` in rendered output instead of one. In ffmpeg drawtext, `%%` → literal `%`, so `%%%%` → `%%`. The replacement should be `%%` to get a single literal `%`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

- Move playlist cover overlay from bottom-right to top-left corner
- Update style guide: image generation produces ONLY the editorial
  artist portrait with no text, graphics, or composited elements
- Playlist covers and logos are overlaid in post-processing via ffmpeg

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
const hasFace = labels.some((label) =>
FACE_LABELS.some((faceLabel) => label.toLowerCase().includes(faceLabel)),
);
logger.log("Face detection result", { imageUrl: imageUrl.slice(0, 80), hasFace, labels });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP - use logStep instead of logger.log

}

if (additionalImageUrls?.length) {
logger.log("Adding additional image URLs", {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP - logStep instead of logger.log

Comment on lines +172 to +184
// Download overlay images to temp files (if any)
const overlayPaths: string[] = [];
if (input.overlayImageUrls?.length) {
logStep("Downloading overlay images", true, { count: input.overlayImageUrls.length });
for (let i = 0; i < input.overlayImageUrls.length; i++) {
const resp = await fetch(input.overlayImageUrls[i]);
if (!resp.ok) continue;
const buf = Buffer.from(await resp.arrayBuffer());
const overlayPath = join(tempDir, `overlay-${i}.png`);
await writeFile(overlayPath, buf);
overlayPaths.push(overlayPath);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP - new lib for generateOverlayPaths.ts

const filterParts: string[] = [];

// Map overlay inputs to their labels
const overlayScaleFilters = overlayImagePaths.map((_, i) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OCP / SRP - move all of this new code to a standalone lib and import it to this file.

"-shortest",
outputPath,
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OCP / SRP - move all this net new code to standalone files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file line count in this file should not exceed 100 lines of code

Comment on lines 33 to 51
let faceGuideUrl: string | null = null;
const additionalImageUrls: string[] = [];

// Upload and classify each provided image
if (images?.length) {
for (const imageUrl of images) {
const uploadedUrl = await fetchImageFromUrl(imageUrl);

if (usesFaceGuide && !faceGuideUrl) {
const hasFace = await detectFace(uploadedUrl);
if (hasFace) {
faceGuideUrl = uploadedUrl;
continue;
}
}

additionalImageUrls.push(uploadedUrl);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP / OCP - new lib for the net new code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still needs implemented.


if (!usesFaceGuide) return null;
// Fall back to GitHub face-guide if needed
if (usesFaceGuide && !faceGuideUrl) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP / OCP - new lib for the net new code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still needs implemented.

sweetmantech and others added 2 commits April 7, 2026 18:30
- Validate usesImageOverlay with Zod schema instead of type assertion
- Cap overlays at 6 to prevent off-frame rendering
- Deduplicate additionalImageUrls against existing image_urls
- Clean up overlay temp files in finally block

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract downloadOverlayImages to its own lib
- Extract fetchGitHubFaceGuide from resolveFaceGuide
- Replace buildOverlayFilters with buildFilterComplex (used by
  renderFinalVideo, removes duplicated overlay positioning logic)
- Use logStep instead of logger.log in detectFace and generateContentImage
- Update detectFace test mock to match logStep import

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 13 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/content/buildFilterComplex.ts">

<violation number="1" location="src/content/buildFilterComplex.ts:48">
P1: `[out]` is not emitted when `captionFilters` is empty, but callers always `-map [out]`, which can break ffmpeg rendering.</violation>
</file>

<file name="src/content/downloadOverlayImages.ts">

<violation number="1" location="src/content/downloadOverlayImages.ts:23">
P1: Per-image download errors are not caught, so one thrown fetch/write failure aborts all remaining overlays instead of skipping failed images.</violation>
</file>

<file name="src/content/generateContentImage.ts">

<violation number="1" location="src/content/generateContentImage.ts:51">
P2: The new dedupe step does not remove duplicates within `additionalImageUrls`; it only excludes URLs already present in `imageUrls`.</violation>
</file>

<file name="src/content/renderFinalVideo.ts">

<violation number="1" location="src/content/renderFinalVideo.ts:222">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

`overlayPaths` is referenced in `finally` out of scope, causing an unresolved identifier build failure.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

const paths: string[] = [];

for (let i = 0; i < urls.length; i++) {
const resp = await fetch(urls[i]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY - we already have an image fetch function right? Why not use it?

"-shortest",
outputPath,
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file line count in this file should not exceed 100 lines of code

sweetmantech and others added 2 commits April 8, 2026 07:38
… escaping bugs

- P0: Add -map 0:a for lipsync audio in overlay path (renderFinalVideo)
- P1: Emit [out] label when captionFilters is empty (buildFilterComplex)
- P1: Wrap per-image download in try/catch (downloadOverlayImages)
- P1: Move overlayPaths declaration before try block (renderFinalVideo)
- P2: Use exact word matching in detectFace to avoid false positives
- P2: Log error details in detectFace catch block
- P2: Self-deduplicate within additionalImageUrls (generateContentImage)
- P2: Fix percent escaping %%%% → %% for single literal % (escapeDrawtext)
- P2: Replace U+02BC with U+2019 for apostrophes — fixes empty box rendering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Moves buildFfmpegArgs, calculateCaptionLayout, stripEmoji, wrapText, and
related constants out of renderFinalVideo.ts into buildFfmpegArgs.ts.
renderFinalVideo.ts now imports and delegates to the extracted module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai bot commented Apr 8, 2026

You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment @cubic-dev-ai review.

/**
* Strips emoji and other non-ASCII characters that ffmpeg drawtext can't render.
*/
function stripEmoji(text: string): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP - all functions should be defined in a file with the same filename as the function name.

  • new file required for stripEmoji

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for all other functions in the file with names different from the filename.

Comment on lines +24 to +28
const resp = await fetch(urls[i]);
if (!resp.ok) continue;
const buf = Buffer.from(await resp.arrayBuffer());
const overlayPath = join(tempDir, `overlay-${i}.png`);
await writeFile(overlayPath, buf);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY - don't we already have a lib to fetch image files? Check if there's an existing lib we can utilize here.

Comment on lines 33 to 51
let faceGuideUrl: string | null = null;
const additionalImageUrls: string[] = [];

// Upload and classify each provided image
if (images?.length) {
for (const imageUrl of images) {
const uploadedUrl = await fetchImageFromUrl(imageUrl);

if (usesFaceGuide && !faceGuideUrl) {
const hasFace = await detectFace(uploadedUrl);
if (hasFace) {
faceGuideUrl = uploadedUrl;
continue;
}
}

additionalImageUrls.push(uploadedUrl);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still needs implemented.


if (!usesFaceGuide) return null;
// Fall back to GitHub face-guide if needed
if (usesFaceGuide && !faceGuideUrl) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still needs implemented.

- SRP: Extract stripEmoji, wrapText, calculateCaptionLayout to own files
- SRP: Extract classifyImages from resolveFaceGuide
- DRY: Extract downloadImageBuffer shared by fetchImageFromUrl and downloadOverlayImages
- Trim renderFinalVideo.ts from 128 to 92 lines

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sweetmantech sweetmantech merged commit 229b7a2 into main Apr 8, 2026
2 checks passed
@sweetmantech sweetmantech deleted the feat/artist-release-editorial-template branch April 8, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants